-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minor fixes in packaging to make it compatible with maturin 1.4.0 #39
Conversation
@microsoft-github-policy-service agree company="Pixelgen Technologies AB" |
Single comment for review left inline, if 3.12 does break it then let's not worry about it now we'll just create an issue to fix it for later :) |
Seems like it is failing right now because maturin's cli args changed since the time I wrote it (which is why I had it pinned to the [0.12..0.13) range, I was tired of breaking changes. Once that is fixed in the github action for building it, we should be good to go though. |
You will also need to bump the version in pyo3's Cargo.toml file to at least 1.2.1 (we can't publish over old versions and the build is set to fail if any of the publication steps fails). I don't think it's really worth trying to make Cargo happy with the pep rules around "1.2.0.post1" as a valid version, and we're not changing anything in the api so 1.2.1 should comply enough with semver to make everyone as happy as they're going to get :) |
4f97985
to
6a62258
Compare
Thank you for the review! I can't seem to find the inline comment you mention. For the other issues I have:
Let me know if there is anything I've missed, or that you'd like done differently. Also, feel free to push any changes you deem necessary to this branch. |
My inline comments have fully disappeared. Let me do this again and check. In the meantime I'm going to approve the workflow and make sure it's working e2e. Edit: sorry about sending you on a wild goose chase re: the inline comments. I don't know what happened! I hope you didn't spend too much time searching for them 😬 |
Writing out the sha256 checksum for the sdist in `dist/`
We should probably support 3.12 as well, since it's a final release. It *should* work, since we're specifically only targeting the broader python 3 abi with this. I should write some simple unit tests that run across the full matrix of python versions. That's a fix for me to do, though.
3.12 support! We think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved!
Thank you for finishing this up! And, no worries about the inline comment - I didn't spend long looking for it. |
1.2.1 is now published to PyPI. https://pypi.org/project/graspologic-native/1.2.1/#files
On Jan 23, 2024, at 11:00 PM, Johan Dahlberg ***@***.***> wrote:
Thank you for finishing this up! And, no worries about the inline comment - I didn't spend long looking for it.
—
Reply to this email directly, view it on GitHub<#39 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAG3SUZJY5LXKDCKLDS3PWLYQCWP5AVCNFSM6AAAAABB47YRX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBXGQ4DGNRZHA>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Thank you! |
I had some issues when trying to build
graspologic-native
, and the following updates seemed to do the trick to get everything working.I basically:
Thank you for providing such a nice package!